Skip to content

Conversation

sebsto
Copy link
Contributor

@sebsto sebsto commented Jul 31, 2025

Do not use a String for Lambdacontext.ClientContext, use a struct instead
fix for #169

Note: this PR introduces an API change that will break function using LambdaContext, we should integrate this change during the beta otherwise it will require a major version bump.

Motivation:

Let the compiler detect type errors for us

Modifications:

  • Create a struct for ClientContext and it's embedded ClientApplication
  • add three unit test to validate the struct

Result:

No more String?

@sebsto sebsto added this to the 2.0 milestone Jul 31, 2025
@sebsto sebsto requested a review from Copilot July 31, 2025 05:29
@sebsto sebsto self-assigned this Jul 31, 2025
@sebsto sebsto added the 🆕 semver/minor Adds new public API. label Jul 31, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR replaces the string-based ClientContext implementation with strongly-typed structs to provide better type safety and compiler error detection for AWS Lambda runtime client context data.

  • Replaces String? type with structured ClientContext and ClientApplication types
  • Adds proper JSON encoding/decoding with snake_case field mapping for AWS compatibility
  • Includes comprehensive unit tests covering various ClientContext scenarios

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
Sources/AWSLambdaRuntime/LambdaContext.swift Introduces ClientContext and ClientApplication structs with proper Codable implementation and updates LambdaContext to use the new types
Tests/AWSLambdaRuntimeTests/LambdaContextTests.swift Adds comprehensive test suite covering JSON encoding/decoding scenarios for the new ClientContext types

@sebsto sebsto requested a review from ktoso July 31, 2025 15:05
@sebsto
Copy link
Contributor Author

sebsto commented Jul 31, 2025

Hello @ktoso
This is the oldest open issue on this project
#169

You opened it 5 years ago :-)
Can you review the change, and provide feedback or approve it ?

Thanks

@sebsto sebsto added ⚠️ semver/major Breaks existing public API. and removed 🆕 semver/minor Adds new public API. labels Jul 31, 2025
@sebsto sebsto enabled auto-merge (squash) August 3, 2025 15:42
Copy link
Contributor

@ktoso ktoso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thank you for addressing this, it's been a while huh!

@sebsto sebsto disabled auto-merge August 4, 2025 06:24
@sebsto sebsto merged commit bae9f27 into swift-server:main Aug 4, 2025
33 of 34 checks passed
@sebsto sebsto deleted the sebsto/clientcontext branch August 4, 2025 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚠️ semver/major Breaks existing public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants